feat: add oclif pretty printer #27

chadian · 2020-06-01T21:37:29Z

closes #28

Adds a prettyPrint function along with an PrettyPrintableInterface with the interface it works with. This is used in the handle function in place of using render methods on Error objects. Pulling out the rendering of an error to a function and not having it be based on an error method allows the prettyPrint function to be used at any point by oclif developers (ie: in a command instance's catch method). It also allows different classes of errors to be pretty printed.

src/handle.ts

src/index.ts

test/error.test.ts

src/errors/cli.ts

src/errors/pretty-print.ts

RasPhilCo

🏄

RasPhilCo

🏄

@rizzlesauce

Thanks @rizzlesauce for your contribution on #19 and highlighting the augmenting the error. Some of those ideas are pulled into this commit. Co-authored-by: Ross Adamson <[email protected]>

amphro · 2020-06-25T18:44:16Z

src/errors/pretty-print.ts

+  /**
+   * a suggestion that may be useful or provide additional context
+   */
+  suggestion?: string;


We have found that we typically want multiple suggestions. https://github.com/forcedotcom/cli-packages/blob/develop/packages/command/src/sfdxCommand.ts#L503

amphro · 2020-06-25T18:44:33Z

src/errors/pretty-print.ts

+   * a url to find out more information related to this error
+   * or fixing the error
+   */
+  ref?: string;


amphro · 2020-06-25T19:01:00Z

src/errors/pretty-print.ts

+  // and is like either Error or Warning
+  const formattedHeader = message ? `${errorSuffix || 'Error'}: ${message}` : undefined
+  const formattedCode = code ? `Code: ${code}` : undefined
+  const formattedSuggestion = suggestion ? `Suggestion: ${suggestion}` : undefined


The Salesforce CLI says "Try this:". I like that a little more than "Suggestion:" but I don't care all that much. It might be worth it to have doc weigh-in. Or I guess to say it another way, if the Salesforce CLI would ever want to use this standard oclif error format, we would want to run it by doc.

amphro · 2020-06-25T19:08:17Z

src/errors/cli.ts

+ */
+export interface OclifError {
+  oclif: {
+    exit?: number | false;


How come this is in an oclif property but the other things like ref, code etc are on the actual error object? I always thought the exitCode should be on the error but a personal opinion.

In any case, this is our error object. Some stuff is specific to Salesforce like labels, but having the properties like actions vs. suggestion would be nice.

This was more for backwards compatibility with the existing structure of cli error and the oclif property that was defined prior. Then the handle function expects it. It's unlikely people would have a different version of the @oclif/errors or would be creating their own errors with oclif.exit but we didn't want to risk it. Instead we wanted to call out it as an oclif error concern one that isn't involved with the display of the error. With your exitCode are you using a custom handle function in bin/run, too?

erezrokah · 2020-11-03T16:11:54Z

FYI this was a breaking change as fs-extra@^9 dropped support for node 8:
https://github.com/jprichardson/node-fs-extra/blob/master/CHANGELOG.md#900--2020-03-19

chadian force-pushed the pretty-print-errors branch 3 times, most recently from e1ec000 to cdb0028 Compare June 4, 2020 19:26

chadian changed the title ~~Add oclif pretty printer~~ feat: add oclif pretty printer Jun 4, 2020

chadian force-pushed the pretty-print-errors branch 3 times, most recently from 1a49ffe to abd450a Compare June 5, 2020 17:50

chadian requested a review from RasPhilCo June 5, 2020 18:02